Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

update angular-mocks.js url matching #12762

Closed
wants to merge 3 commits into from

Conversation

deodeveloper
Copy link
Contributor

url matching was incorrect and updated. if all query parameters in url are in different order, it should still match

url matching was incorrect and updated. if all query parameters in url are in different order, it should still match
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@deodeveloper
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Sep 4, 2015
@petebacondarwin
Copy link
Contributor

Thanks for this @deodeveloper - before we can land this change it would need to have a unit test to show that it is doing what you expect.

@deodeveloper
Copy link
Contributor Author

For merging with master, updated a test case for the changes related to this the pull request as commented by @petebacondarwin

@gkalpak
Copy link
Member

gkalpak commented Feb 1, 2016

There are some stying issues (e.g. see https://travis-ci.org/angular/angular.js/jobs/106143760).
You can also run the ci-checks locally (grunt ci-checks) - I believe there will be some more (e.g. whitespace between function parameters) once you fix the semicolon issues 😉

@@ -1646,6 +1646,15 @@ function MockHttpExpectation(method, url, data, headers) {
}
return data == d;
};

this.getUrlParams = function (u){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these two methods (getUrlParams and compareUrl) need to be public ?

@gkalpak
Copy link
Member

gkalpak commented Feb 1, 2016

This doesn't properly support URLs with a hash fragment, but I guess we don't really care, because it doesn't make sense to make requests against URLs with a hash fragment (since hash fragments have no meaning for the server).
(In any case, this PR doesn't make them any less supported than they already were, I just mention it as a possible (probably unnecessary) enhancement.)

@petebacondarwin
Copy link
Contributor

@deodeveloper can you address the issues that @gkalpak has highlighted?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants